Fixed singleton design to fix issues with async initialization. - #21
Fixed singleton design to fix issues with async initialization.#21darthdie wants to merge 1 commit into
Conversation
|
did you have a problem with the |
I did. I'll open a separate PR with a quick fix. Edit: see #34 |
| await synchronized(_lock, () async { | ||
| if (_instance == null) { | ||
| _instance = new CacheManager._(); | ||
| await _instance._init(); |
There was a problem hiding this comment.
@darthdie @mockturtl , an alternative fix might be to do something like:
final backingInstance = new CacheManager._();
await backingInstance.init();
_instance = backingInstance;
Delaying the assignment to _instance until after the initialization keeps that initial null guard working as designed.
This way we don't have to worry about initializing _cacheData in #34 (and then re-initializing it with the new Map() call), and then you don't end up with a race condition on which map (initial or re-initialized) is hit.
Hm. After thinking about it, I'm confused as to how #34 happens with this PR (original version) in place. AFAICT, it shouldn't?
Anyway, just some thoughts. I do like how this current PR gets rid of the nested null guards, and I mostly started to write this out of interest on whether the _cacheData initialization is necessary.
I was having issues when my app was loading multiple images at the same time on start-up.
Should also resolve the issues you've mentioned before with
synchronizednot working as you thought with singletons.